svengato intermine-ws-python enhancements#55
Conversation
yochannah
left a comment
yochannah
left a comment
There was a problem hiding this comment.
Hi @svengato - thanks so much for making this PR! Adding the title and long description makes sense, but it might be handy if you could explain what exactly the other changes are intended to fix and how to test it. Once it is tested & verified okay we'd be happy to merge! :)
|
p.s. - it is possible to fix the pep8 issues using the pep8 auto fix command in the readme. https://github.com/intermine/intermine-ws-python#before-making-prs I also notice it's your first PR on GitHub - congrats! If you are thinking to make a few more (to InterMine or elsewhere) you coud sign up for hacktoberfest and get a free tshirt: https://hacktoberfest.digitalocean.com/ |
|
@svengato thanks for updating this to work nicely with PEP8! It looks like there are a couple of test failures - the new error text you wrote is being output in our logs: https://travis-ci.org/intermine/intermine-ws-python/jobs/598365963#L3101 Are you able to take a look? |
|
I just noticed that Template is a subclass of Query, so it seems more logical to handle parsing the template title there. Let me experiment with that and get back to you - |
|
It was a bit tricky as from_xml() is a Query class method, but this version should not generate the same error when parsing a non-template query. |
3 participants
I am using the intermine python module to return templates from multiple InterMines. These changes are required to extract all the information I need.